Skip to content

Twister: Sharing built applications across multiple tests #75291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Twister: Sharing built applications across multiple tests #75291

wants to merge 4 commits into from
+402 −128

Conversation

@gchwier
Copy link
Contributor

@gchwier gchwier commented Jul 1, 2024 •
edited
Loading

Related to #62649
and @gopiotr 's PR: #66188

Added sample to demonstrate sharing build mechanism in samples/subsys/testsuite/shared_app,
try to run with:

./scripts/twister -T samples/hello_world -T samples/subsys/testsuite/shared_app -v -p native_sim -p qemu_cortex_m3

Changes:

  • added required_images key to sample/testcase yaml. It is a list of test scenarios that are required to be build before current test. If required image is not ready, then processing of the current test is postponed (received to the pipeline queue). There is also option to use the first required image as an image used during current test (user must set new key no_own_image or there is no source (CMakeList.txt) in the current test directory)
  • most of the changes are in runner.py file. pipeline queue is changed from LifoQueue to deque (to allow append elements in both sides), done is changed from LifoQueue to dict (to check if test scenario is ready with constant time).
  • in testplan.py checked if required images are in scope, if platform is correct etc. Added special filter to mark filtered test scenario with proper comments
  • updated unit tests and added new keys to twister doc

This feature does not work with --subset.
For QEMU platforms there is no support for no_own_image (QEMU_PIPE file is used during build, so to reuse qemu build, one must point to the origin build directory).

All required images must be available from tree (user must give path with -T option). If required image is not found in scope, then is skipped.

Usecases:

  1. as provided in [RFC] Sharing built applications across multiple tests #62649 with bluetooth samples
  2. I want to test DFU and in many cases I can use another image during upgrade (e.g. hello_world or smp_svr) to test downgrade prevention mechanism, wrong keys, wrong encryption used for MCUboot etc. (from the NCS perspective, a lot of tests with B0 bootloader)
  3. just to test another sample without copying it. I want to test e.g. smp_svr with pytest scenarios. With one sample, without modifying it and rebuilding, I can run many pytest testcases

@gchwier gchwier requested review from aescolar and gopiotr July 1, 2024 17:14
@gchwier gchwier force-pushed the grch_required_images branch 4 times, most recently from 798f60b to c2d9295 Compare July 2, 2024 15:09
@nashif nashif added this to the v4.0.0 milestone Jul 2, 2024
@gchwier gchwier force-pushed the grch_required_images branch 3 times, most recently from f40b3ff to a421250 Compare July 2, 2024 22:02
hakehuang
hakehuang reviewed Jul 3, 2024
View reviewed changes
@hakehuang
Copy link
Contributor

hakehuang commented Jul 3, 2024

and the atomic pacakge is other application right?

@gchwier gchwier force-pushed the grch_required_images branch from a421250 to a45ff5f Compare July 4, 2024 13:47
@gchwier
Copy link
Contributor Author

gchwier commented Aug 6, 2024

updated with changes to properly add required images when tests are loaded from file (options --test-only, --load-tests, --only-failed)

@aescolar
Copy link
Member

aescolar commented Aug 9, 2024 •
edited
Loading

Right now, if the required image does not exist and is not in the plan, the dependent test will just be skipped. But wouldn't it be more intuitive and user friendly if a test's required images were built/added to the queue if missing from the plan?

I agree that it would be more user friendly, but for now, I wanted only provide a "simple" method to be able to reuse other images

Fair enough. Let's go with this by now then.
Later, users will be very even happier when that extra is added.

aescolar
aescolar previously approved these changes Aug 9, 2024
View reviewed changes
Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 nits. Overall +1 to the idea

@gchwier gchwier force-pushed the grch_required_images branch from b63cd17 to 867b3ff Compare August 9, 2024 12:20
@gchwier
Copy link
Contributor Author

gchwier commented Aug 9, 2024

Thank you @aescolar for review. I rebased PR and updated twister.rst with your comments.

Another question, if I run with --no-clean -c I don't get tests which already existed reused(?)

Now I know what you mean. It could be great to be able to run
twister --build-only <all_required_images>
and then:
twister -T <image_that_requires_another_images> ... --no-clean
For now it is not possible. You must have every required image in test plan, so you must include it to the command. It is not enough to have it built in twister-out dir.
I will try to add this feature later (some changes are required in testplan.py to find image in twister-out dir and to process them in runner.py), but in separate PR.

@gchwier gchwier requested a review from LukaszMrugala August 9, 2024 12:45
@gchwier gchwier force-pushed the grch_required_images branch 2 times, most recently from f1cee35 to 630c36d Compare August 13, 2024 10:38
@aescolar aescolar mentioned this pull request Aug 16, 2024
Closed
@gchwier gchwier force-pushed the grch_required_images branch from 630c36d to bde485c Compare August 19, 2024 13:04
@gchwier
Copy link
Contributor Author

gchwier commented Aug 20, 2024

Rebased to work with TwisterStatus.
@LukaszMrugala Could you please review the changes once again? Thank you!

@nashif
Copy link
Member

nashif commented Aug 20, 2024

@gchwier

this basically does what I commented on in #62649 (comment), correct?

@gchwier
Copy link
Contributor Author

gchwier commented Aug 20, 2024

this basically does what I commented on in #62649 (comment), correct?

Yes, correct. This change modifies the scheduler and queue management. Instead of using LifoQueue, it now uses deque and dict (registered with a BaseManager to be shared between multiple processes).

  • dict is used for done instances to quickly check if the processing of required testinstances is finished.
  • deque is used for pipeline instances to allow postponing the processing of a task (re-adding the task to the front of the queue) if the required instances have not been processed yet.

@gchwier
twister: scripts: implement sharing build mechanism
07a1e8e 
Added feature to share builds between test scenarios in Twister.
New keys added to test yaml: required_images and no_own_image.

Signed-off-by: Grzegorz Chwierut <[email protected]>
@gchwier
twister: unittests: align tests with changes for shared apps
ca1b6bc 
Fixed unittests after adding shared apps feature.

Signed-off-by: Grzegorz Chwierut <[email protected]>
@gchwier
samples: added sample to demonstrate new Twister feature.
9ddfa62 
Added sample to demonstrate shared app mechanism added
to Twister.

Signed-off-by: Grzegorz Chwierut <[email protected]>
@gchwier
doc: twister: Added required_images key description.
dab9a22 
Added short description of new keys added with shared
app mechanism.

Signed-off-by: Grzegorz Chwierut <[email protected]>
@gchwier gchwier force-pushed the grch_required_images branch from bde485c to dab9a22 Compare September 23, 2024 14:50
@gchwier
Copy link
Contributor Author

gchwier commented Sep 23, 2024 •
edited
Loading

Rebased and changed names of variables used by scheduler:
done_queue to ready_instances
pipeline to processing_queue
@nashif Is the change in scheduler and queue management a blocker for this PR? (ref. comment #62649 (comment))

@mmahadevan108 mmahadevan108 modified the milestones: v4.0.0, v4.1.0 Nov 8, 2024
@github-actions
Copy link

github-actions bot commented Jan 8, 2025

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jan 8, 2025
@github-actions github-actions bot closed this Jan 23, 2025
@aescolar
Copy link
Member

aescolar commented Jan 23, 2025 •
edited
Loading

Oh well, it is really a pity this has not got anywhere so far.
@gchwier @nashif what would need to happen for this to be merged?

@gchwier
Copy link
Contributor Author

gchwier commented Jan 28, 2025

Rebase is not trivial now (changes related to statuses, ruff etc.). I can do that, however as I understand, tinkering in Twister scheduler and queue management was a blocker here. (#75291 (comment))

@aescolar aescolar mentioned this pull request Feb 11, 2025
Closed
@PerMac PerMac mentioned this pull request Apr 7, 2025
Open
@gchwier gchwier mentioned this pull request Aug 6, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hakehuang hakehuang hakehuang left review comments

@aescolar aescolar aescolar left review comments

@aaronemassey aaronemassey Awaiting requested review from aaronemassey

@asemjonovs asemjonovs Awaiting requested review from asemjonovs

@carlescufi carlescufi Awaiting requested review from carlescufi

@golowanow golowanow Awaiting requested review from golowanow

@jeremybettis jeremybettis Awaiting requested review from jeremybettis

@KamilxPaszkiet KamilxPaszkiet Awaiting requested review from KamilxPaszkiet

@kartben kartben Awaiting requested review from kartben

@nashif nashif Awaiting requested review from nashif

@PerMac PerMac Awaiting requested review from PerMac

@yperess yperess Awaiting requested review from yperess

@gopiotr gopiotr Awaiting requested review from gopiotr

@LukaszMrugala LukaszMrugala Awaiting requested review from LukaszMrugala

Assignees

@nashif nashif

Labels

area: Documentation area: Samples Samples area: Testsuite Testsuite area: Twister Twister Stale

Projects

None yet

Milestone

v4.1.0

Development

Successfully merging this pull request may close these issues.

7 participants

@gchwier @hakehuang @aescolar @nashif @LukaszMrugala @mmahadevan108 @zephyrbot